Skip to content

chore: merge workflow agent config for resource selection#1163

Merged
adityachoudhari26 merged 1 commit into
mainfrom
merge-workflow-agent-config
Jun 5, 2026
Merged

chore: merge workflow agent config for resource selection#1163
adityachoudhari26 merged 1 commit into
mainfrom
merge-workflow-agent-config

Conversation

@adityachoudhari26

@adityachoudhari26 adityachoudhari26 commented Jun 5, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • Bug Fixes

    • Fixed job dispatcher to correctly apply merged per-job configuration when routing resources across agents.

  • Tests

    • Added test validating proper resource routing with per-job configuration settings.

Copilot AI review requested due to automatic review settings June 5, 2026 15:22
@coderabbitai

coderabbitai Bot commented Jun 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0679c1a1-608c-4113-82d7-b3814c6cb95a

📥 Commits

Reviewing files that changed from the base of the PR and between 81ac42e and 44b4d1e.

📒 Files selected for processing (2)
  • apps/workspace-engine/svc/http/server/openapi/workflows/dispatch.go
  • apps/workspace-engine/svc/http/server/openapi/workflows/workflows_test.go
📝 Walkthrough

Walkthrough

The PR modifies planDispatches to use merged per-job job-agent configuration when matching agents to resources. The implementation creates a local copy of the job-agent and overwrites its config with the merged configuration before selector matching. A new test validates that each dispatch routes to the correct per-job server URL from the merged config.

Changes

Merged Job-Agent Config Routing

Layer / File(s) Summary
Merged config in dispatch selector matching
apps/workspace-engine/svc/http/server/openapi/workflows/dispatch.go, apps/workspace-engine/svc/http/server/openapi/workflows/workflows_test.go		 planDispatches copies dispatchCtx.JobAgent to a local matchAgent and overwrites its Config with mergedConfig before passing to selector.MatchJobAgentsWithResource. Test TestPlanDispatches_RoutesOnMergedPerJobConfig verifies two job-agent entries with distinct serverUrl values each route to their corresponding merged per-job server.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • ctrlplanedev/ctrlplane#1098: Merges runner config into WorkflowJobAgent.Config so per-job values like serverUrl are available for this PR's selector matching.
  • ctrlplanedev/ctrlplane#1103: Applies similar merged per-job config logic to dispatchJobForAgent to ensure consistent use of merged configuration across dispatch operations.

Suggested reviewers

  • dacbd

Poem

A rabbit hops through merged configs with glee, 🐰
Each job now routes where it's meant to be,
No more lost serverUrls in the fray,
Dispatch selectors choose the right way! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'chore: merge workflow agent config for resource selection' directly matches the main change: ensuring planDispatches uses merged per-agent configuration for selector matching.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch merge-workflow-agent-config

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI reviewed Jun 5, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes workflow job-agent routing so that resource-selection selectors evaluate against the merged agent configuration (runner/base config + per-job overrides), enabling correct dispatch when key routing fields (e.g., serverUrl) are only present in per-job config.

Changes:

  • Update planDispatches to pass an oapi.JobAgent whose Config is the merged config into MatchJobAgentsWithResource.
  • Add a regression test ensuring resources route correctly when serverUrl is supplied only via per-job config entries.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
apps/workspace-engine/svc/http/server/openapi/workflows/dispatch.go Ensure selector matching uses merged job-agent config (not only runner config).
apps/workspace-engine/svc/http/server/openapi/workflows/workflows_test.go Add coverage for routing based on merged per-job config.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@adityachoudhari26 adityachoudhari26 merged commit 1b6e0d9 into main Jun 5, 2026
11 checks passed
@adityachoudhari26 adityachoudhari26 deleted the merge-workflow-agent-config branch June 5, 2026 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@adityachoudhari26